Skip to content

Simplify and improve trackingAllocationSplit() #1254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Apr 11, 2025
Merged

Simplify and improve trackingAllocationSplit() #1254

merged 1 commit into from
Apr 11, 2025

Conversation

@ldorau
Copy link
Contributor

@ldorau ldorau commented Apr 11, 2025 •
edited
Loading

Description

We do not need to allocate a new splitValue, update critnib using critnib_insert() in the update mode and free the old value. It is enough to just update atomically the size of the first part:

utils_atomic_store_release_u64((uint64_t *)&value->size, firstSize);

Also add missing umfMemoryProviderAllocationMerge() call in the error handling path.

Fixes: #1244

Checklist

  • Code compiles without errors locally
  • All tests pass locally
  • CI workflows execute properly

@ldorau
Simplify and improve trackingAllocationSplit()
6cd4759 
We do not need to allocate a new splitValue,
update critnib using critnib_insert() in the update mode
and free the old value. It is enough to just update atomically
the size of the first part:

utils_atomic_store_release_u64((uint64_t *)&value->size, firstSize);

Also add missing umfMemoryProviderAllocationMerge() call
in the error handling path.

Signed-off-by: Lukasz Dorau <[email protected]>
@ldorau ldorau requested a review from a team as a code owner April 11, 2025 06:42
@ldorau
Copy link
Contributor Author

ldorau commented Apr 11, 2025 •
edited
Loading

It fixes also the following data race:
https://github.com/ldorau/unified-memory-framework/actions/runs/14399212255/job/40381426774

WARNING: ThreadSanitizer: data race (pid=12265)
  Write of size 8 at 0x7f11c1700fd8 by thread T24 (mutexes: write M0, write M1, write M2):
    #0 critnib_insert /home/runner/work/unified-memory-framework/unified-memory-framework/src/critnib/critnib.c:361:31 (test_jemalloc_pool+0x16910b) (BuildId: 5e66f681b3ac2e39585494c392892b690211aaa4)
    #1 trackingAllocationSplit /home/runner/work/unified-memory-framework/unified-memory-framework/src/provider/provider_tracking.c:552:9 (test_jemalloc_pool+0x1652af) (BuildId: 5e66f681b3ac2e39585494c392892b690211aaa4)
    #2 umfMemoryProviderAllocationSplit /home/runner/work/unified-memory-framework/unified-memory-framework/src/memory_provider.c:332:24 (test_jemalloc_pool+0x15a17c) (BuildId: 5e66f681b3ac2e39585494c392892b690211aaa4)
    #3 arena_extent_split /home/runner/work/unified-memory-framework/unified-memory-framework/src/pool/pool_jemalloc.c:274:12 (test_jemalloc_pool+0x16c946) (BuildId: 5e66f681b3ac2e39585494c392892b690211aaa4)

  Previous read of size 8 at 0x7f11c1700fd8 by thread T21 (mutexes: write M3):
    #0 critnib_find /home/runner/work/unified-memory-framework/unified-memory-framework/src/critnib/critnib.c:763:26 (test_jemalloc_pool+0x16a9f5) (BuildId: 5e66f681b3ac2e39585494c392892b690211aaa4)
    #1 umfMemoryTrackerAdd /home/runner/work/unified-memory-framework/unified-memory-framework/src/provider/provider_tracking.c:196:13 (test_jemalloc_pool+0x166c5a) (BuildId: 5e66f681b3ac2e39585494c392892b690211aaa4)
    #2 trackingAlloc /home/runner/work/unified-memory-framework/unified-memory-framework/src/provider/provider_tracking.c:481:11 (test_jemalloc_pool+0x1641e7) (BuildId: 5e66f681b3ac2e39585494c392892b690211aaa4)
    #3 umfMemoryProviderAlloc /home/runner/work/unified-memory-framework/unified-memory-framework/src/memory_provider.c:245:9 (test_jemalloc_pool+0x1597be) (BuildId: 5e66f681b3ac2e39585494c392892b690211aaa4)
    #4 arena_extent_alloc /home/runner/work/unified-memory-framework/unified-memory-framework/src/pool/pool_jemalloc.c:105:11 (test_jemalloc_pool+0x16c3c7) (BuildId: 5e66f681b3ac2e39585494c392892b690211aaa4)

@ldorau
Copy link
Contributor Author

ldorau commented Apr 11, 2025

Fixes: #1244

@ldorau ldorau mentioned this pull request Apr 11, 2025
Closed
@ldorau ldorau requested review from PatKamin and bratpiorka April 11, 2025 09:25
@lplewa lplewa merged commit 6c576c1 into oneapi-src:main Apr 11, 2025
87 checks passed
@ldorau ldorau deleted the Simplify_and_improve_trackingAllocationSplit branch April 11, 2025 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lplewa lplewa lplewa approved these changes

@PatKamin PatKamin PatKamin approved these changes

@bratpiorka bratpiorka Awaiting requested review from bratpiorka

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Race in jemalloc pool multithreaded test

3 participants

@ldorau @lplewa @PatKamin